-
Notifications
You must be signed in to change notification settings - Fork 31
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Bring jcb into GDASapp and use version of jcb not using submodules #1146
Conversation
* develop: Update wcoss2.intel.lua (#1142) Add yaml file for prepobsaero task (#1141) Diffusion parameter yaml template (#1140) Adjust absolute float tolerance for variational and ensemble DA jjob tests (#1136) Insitu temp and salt (#1135) Adds letkf.yaml(.j2) (#1134) Add optional testing block to 3dvar and lgetkf input YAMLs for jjobs (#1129) updates to build and run some ctests on WCOSS2 (#1122) Addin a GFSv17 ctest (#1130) Feature/rtofs in situ (#1110) update jcb-gdas hash to bring in satwnd yaml changes (#1128) Optionally run specified rocoto task as part of the ctests (#1121) Add build capability to Gaea-C5 (#1101) Python Converter and Json for the ADPUPA BUFR DUMP (#1115) Revert "Fix ice water FV3 increment variable name and add a few more … (#1125) Satwnd and variable name-convention updates for jcb implementation in GDASApp (#1119) Add inverse distance weighting option to superob function (#1116) Resume the Marine Vrfy Task on ctests (#1107) Replicate the creation of the gw-ci cycling test (#1114) Added YAML, JSON, python files for assimilating LEOGEO satwinds (#1099) Add an OOPS-based application to recenter snow analysis (#1102) Fix ice water FV3 increment variable name and add a few more (#1112) Update hera.intel.lua (#1109)
@danholdaway , the changes in GDASApp branch However, I now see a problem. g-w PR #2655 merges your forked g-w branch danholdaway:feature/move_jcb into g-w I think we want g-w |
That is correct @RussTreadon-NOAA. But as soon as we merge GDASapp everything will be broken while we wait for the GW PR to be merged. Alternatively we can merge G-W and then GDASapp and then make another g-w PR changing the hash to develop. That’s the only way that everything continues to work. What do you think? Is that acceptable? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tested as part of g-w #2565. Looks good.
Approve.
@danholdaway , OK. I see your logic. I never thought of having g-w point at a GDASApp branch instead of Your approach allows us to bypass the sequential method I've been using: GDASApp first and then g-w. g-w PR #2665 can move ahead as is. After this GDASApp PR is merged into GDASApp |
@RussTreadon-NOAA perhaps in this instance we merge down from g-w to gdas to jcb. Just because it takes longest to get something into g-w wheres we can more quickly merge the others. And in terms of the code pointed to in the hashes it would be equivalent |
if it's an unchanged squash commit, is the hash identical in the branch and after merged to develop? |
Works for me. Just want to get as many of your changes into |
No because you have to squash all the commits with a new hash. But the code that gets run would be identical I think. |
Lets say we merge g-w, then gdas then jcb. Then immediately merged upward jcb, gdas, g-w so that the hashes were the hashes develop points to for each repo...I think the latter would be a sequence of empty PRs. In the g-w case they might even reject the PR, as Rahul did when I tried to bump the hash of GSI utils despite no changes. I might actually try that to see if that theory is correct after these are merged. For non-breaking changes it wouldn't matter which way you merged. |
Automated GDASApp Testing Results:
|
Automated Global-Workflow GDASApp Testing Results:
|
Orion test
|
@guillaumevernieres , given successful CI on Hera (automated tests) and Orion, I'd like to merge this PR into |
sounds good @RussTreadon-NOAA |
This PR has two companion PRs Both have already been approved by @CoryMartin-NOAA. Given this, we should merge these jcb PRs as part of merging this PR. Note that |
Bring jcb into the GDASapp repo and use the versions of jcb that do not use submodules:
There is a GW branch of the same name and I will make a PR there once this is merged. https://github.com/danholdaway/global-workflow/tree/feature/move_jcb